Add support for Network Tier configuration#15083
Add support for Network Tier configuration#15083melinath merged 10 commits intoGoogleCloudPlatform:mainfrom
Conversation
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing doc report (experimental)The following resources have fields missing in documents.
|
Non-exercised tests🔴 Tests were added that are skipped in VCR:
Tests analyticsTotal tests: 0 Click here to see the affected service packages
🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR. View the build log |
f3e2e4d to
d5c98ad
Compare
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing doc report (experimental)The following resources have fields missing in documents.
|
Non-exercised tests🔴 Tests were added that are skipped in VCR:
Tests analyticsTotal tests: 0 Click here to see the affected service packages
🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR. View the build log |
d5c98ad to
4b282a9
Compare
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing doc report (experimental)The following resources have fields missing in documents.
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: For automatic test runs see go/terraform-auto-test-runs. @melinath, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
Test failures analysisSome of the VCR tests are failing after my changes, but all of them seem unrelated to the changes I am making in this PR. Please look at the following breakdown. Recording mode
Replaying mode
|
1810cbc to
3acca55
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Test failures analysisI pushed changes to solve a documentation conflict, as such, all of these failures seem unrelated to the changes, as the code hasn't been touched. Additionally, there seems to be a quota issue as most failures seem related to the project hitting a count limit or being unable to reserve IP addresses. |
|
Thanks for fixing the tests! Looks like the test I introduced broke, I will take a look into that. |
Tests analyticsTotal tests: 260 Click here to see the affected service packages
Action takenFound 19 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
This comment was marked as outdated.
This comment was marked as outdated.
|
@melinath The fix is done! The failing tests seem unrelated to the changes, as they have been failing in multiple PRs for a while now. PTAL. Thanks!! |
|
@GoogleCloudPlatform/terraform-team @melinath This PR has been waiting for review for 1 week. Please take a look! Use the label |
|
Fixed more of the GKE issues. Triggering a new run to record cassettes for this PR. Sorry for the noise! /gcbrun |
Tests analyticsTotal tests: 260 Click here to see the affected service packages
Action takenFound 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
🟢 Tests passed during RECORDING mode: 🟢 No issues found for passed tests after REPLAYING rerun. 🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
|
Hello maintainers. Thanks for the effort cleaning up the tests. It seems like the remaining failures are still unrelated. PTAL. |
melinath
left a comment
There was a problem hiding this comment.
overall looks good; just a few small change suggestions.
| Optional: true, | ||
| Default: "NETWORK_TIER_DEFAULT", |
There was a problem hiding this comment.
I'd recommend marking this as Required: true so that if the network_tier_config block is specified, users must provide a value for network_tier.
Generally we try to avoid allowing empty blocks (unless they have a special meaning separate from the block being omitted.)
| Optional: true, | |
| Default: "NETWORK_TIER_DEFAULT", | |
| Required: true, |
| Type: schema.TypeString, | ||
| Optional: true, | ||
| Default: "NETWORK_TIER_DEFAULT", | ||
| ValidateFunc: validation.StringInSlice([]string{"NETWORK_TIER_DEFAULT", "NETWORK_TIER_STANDARD", "NETWORK_TIER_PREMIUM"}, false), |
There was a problem hiding this comment.
If there's a chance that more values will be added in the future, you might want to leave off the validation func so that the field will be forwards-compatible.
mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.tmpl
Show resolved
Hide resolved
mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.tmpl
Show resolved
Hide resolved
printchard
left a comment
There was a problem hiding this comment.
Thanks for the feedback. I have made the requested changes, PTAL.
| Optional: true, | ||
| Default: "NETWORK_TIER_DEFAULT", |
mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.tmpl
Show resolved
Hide resolved
mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.tmpl
Show resolved
Hide resolved
| Type: schema.TypeString, | ||
| Optional: true, | ||
| Default: "NETWORK_TIER_DEFAULT", | ||
| ValidateFunc: validation.StringInSlice([]string{"NETWORK_TIER_DEFAULT", "NETWORK_TIER_STANDARD", "NETWORK_TIER_PREMIUM"}, false), |
Tests analyticsTotal tests: 260 Click here to see the affected service packages
Action takenFound 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
|
Failing tests look unrelated. |
ad90670
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.